[DO NOT MERGE] flaky test investigation #3364
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
||
| return undefined; | ||
| } | ||
| @service('intl') declare intl: IntlService; |
There was a problem hiding this comment.
@zamoore I think this is how we are supposed to do it but I'm not 100%, worth a check though.
There was a problem hiding this comment.
Does this cause errors in codebases where ember-intl is not installed?
There was a problem hiding this comment.
It would have, moved ember-intl from devDep to Dep for the components package, should be fine.
There was a problem hiding this comment.
I am still not sure how we'd actually test this w/o creating an engines situation...
There was a problem hiding this comment.
I only tested in cloud-ui through RC, it only viable option without spinning another test app with engines, I guess we could create a failing test 🤔 in the showcase
There was a problem hiding this comment.
I think we should be able to do an ember-try scenario
There was a problem hiding this comment.
@shleewhite since we talked about it today I didn't make a lot of changes BUT a lot of tests that were consistently failing stopped failing after adding the destroyed/destroying check.
| </Hds::AdvancedTable>`; | ||
|
|
||
| const hbsResizableColumnsAdvancedTable = hbs`<div style="width: 1000px;"> | ||
| const hbsResizableColumnsAdvancedTable = hbs`{{!-- template-lint-disable no-inline-styles --}} |
There was a problem hiding this comment.
We're getting a lot of errors in our showcase and tests because we aren't taking the time to obey the linting rules or do types properly. Maybe this gets fixed with the refactor so we should just wait on it, but I've noted a few things just in case.
| ); | ||
| await focus(firstThElement); | ||
| await focus(firstReorderHandle); | ||
| // need to flush the frame to let the RAF waiter finish doing its thing |
There was a problem hiding this comment.
I think this is what we need but also not sure why we're finding three things in the DOM and then focusing on two things in direct succession before asserting anything. Do we need the first focus? Could we find and focus the firstReorderHandle directly?
| if (reorderedMessageText !== undefined) { | ||
| this.reorderedMessageText = reorderedMessageText; | ||
| } else { | ||
| } else if (!isDestroyed(this) && !isDestroying(this)) { |
There was a problem hiding this comment.
fwiw: I tried a similar approach a while back and it seemed to solve the issue with hdsIntl service
5 participants
Summary
Spelunking in our codebase to see if I can figure out the causes of, and solutions for, some of our flaky tests.
Details
Specific things that I've done as I've identified test errors (and tried to fix) and other changes/refactoring to try to reduce test flakiness:
@embroider/macrosfor the link-to-external utilgetin the intl service with a direct injection of the intl service (so that anything that uses our intl service is more stable and teardown safe due to delegation of ownership to the framework)ember-intlfrom devDep to Dep for component package.isDestroyed/isDestroyingguard/check to advanced table itselfwaitForfrom ember-test-helpersMore info
hds-resolve-link-to-externalutil to be something likeoptionalLinkToExternal.gtswhich would remove the util and tighten up the component code that uses this util by quite a bit. But we should discuss.